From 2d25fcacc808bbbedb33d1f42a764ecdaf6615fb Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 19 Feb 2016 10:53:57 -0800 Subject: [PATCH] Fuse SourceMap and PackageSet This commit moves the SourceMap structure into the PackageSet structure, and simultaneously massively cuts down on the API surface area of PackageSet. It's intended that eventually a PackageSet will be a lazily loaded set of packages so we don't have to download everything all at once, and this is the commit in preparation of that. --- src/cargo/core/package.rs | 81 ++++------------------- src/cargo/core/registry.rs | 13 ++-- src/cargo/ops/cargo_clean.rs | 30 ++------- src/cargo/ops/cargo_compile.rs | 16 ++--- src/cargo/ops/cargo_fetch.rs | 16 +++-- src/cargo/ops/cargo_output_metadata.rs | 28 ++++---- src/cargo/ops/cargo_rustc/context.rs | 24 ++----- src/cargo/ops/cargo_rustc/custom_build.rs | 2 +- src/cargo/ops/cargo_rustc/fingerprint.rs | 2 +- src/cargo/ops/cargo_rustc/layout.rs | 12 +++- src/cargo/ops/cargo_rustc/links.rs | 2 +- src/cargo/ops/cargo_rustc/mod.rs | 15 +++-- 12 files changed, 84 insertions(+), 157 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 9aa3c8669..4df81b162 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -5,11 +5,11 @@ use std::slice; use std::path::{Path, PathBuf}; use semver::Version; -use core::{Dependency, Manifest, PackageId, SourceId, Registry, Target, Summary, Metadata}; +use core::{Dependency, Manifest, PackageId, SourceId, Target}; +use core::{Summary, Metadata, SourceMap}; use ops; -use util::{CargoResult, graph, Config}; +use util::{CargoResult, Config}; use rustc_serialize::{Encoder,Encodable}; -use core::source::Source; /// Information about a package that is available somewhere in the file system. /// @@ -118,78 +118,25 @@ impl hash::Hash for Package { } } -#[derive(PartialEq,Clone,Debug)] -pub struct PackageSet { +pub struct PackageSet<'cfg> { packages: Vec, + sources: SourceMap<'cfg>, } -impl PackageSet { - pub fn new(packages: &[Package]) -> PackageSet { - //assert!(packages.len() > 0, - // "PackageSet must be created with at least one package") - PackageSet { packages: packages.to_vec() } - } - - pub fn is_empty(&self) -> bool { - self.packages.is_empty() - } - - pub fn len(&self) -> usize { - self.packages.len() - } - - pub fn pop(&mut self) -> Package { - self.packages.pop().expect("PackageSet.pop: empty set") - } - - /// Get a package by name out of the set - pub fn get(&self, name: &str) -> &Package { - self.packages.iter().find(|pkg| name == pkg.name()) - .expect("PackageSet.get: empty set") - } - - pub fn get_all(&self, names: &[&str]) -> Vec<&Package> { - names.iter().map(|name| self.get(*name) ).collect() - } - - pub fn packages(&self) -> &[Package] { &self.packages } - - // For now, assume that the package set contains only one package with a - // given name - pub fn sort(&self) -> Option { - let mut graph = graph::Graph::new(); - - for pkg in self.packages.iter() { - let deps: Vec<&str> = pkg.dependencies().iter() - .map(|dep| dep.name()) - .collect(); - - graph.add(pkg.name(), &deps); +impl<'cfg> PackageSet<'cfg> { + pub fn new(packages: Vec, + sources: SourceMap<'cfg>) -> PackageSet<'cfg> { + PackageSet { + packages: packages, + sources: sources, } - - let pkgs = match graph.sort() { - Some(pkgs) => pkgs, - None => return None, - }; - let pkgs = pkgs.iter().map(|name| { - self.get(*name).clone() - }).collect(); - - Some(PackageSet { - packages: pkgs - }) } - pub fn iter(&self) -> slice::Iter { + pub fn packages(&self) -> slice::Iter { self.packages.iter() } -} -impl Registry for PackageSet { - fn query(&mut self, name: &Dependency) -> CargoResult> { - Ok(self.packages.iter() - .filter(|pkg| name.name() == pkg.name()) - .map(|pkg| pkg.summary().clone()) - .collect()) + pub fn sources(&self) -> &SourceMap<'cfg> { + &self.sources } } diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index fc994aeaf..f56aaa93a 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -1,7 +1,7 @@ -use std::collections::HashSet; -use std::collections::hash_map::HashMap; +use std::collections::{HashSet, HashMap}; use core::{Source, SourceId, SourceMap, Summary, Dependency, PackageId, Package}; +use core::PackageSet; use util::{CargoResult, ChainError, Config, human, profile}; /// Source of information about a group of packages. @@ -85,7 +85,8 @@ impl<'cfg> PackageRegistry<'cfg> { } } - pub fn get(&mut self, package_ids: &[PackageId]) -> CargoResult> { + pub fn get(mut self, package_ids: &[PackageId]) + -> CargoResult> { trace!("getting packages; sources={}", self.sources.len()); // TODO: Only call source with package ID if the package came from the @@ -104,11 +105,7 @@ impl<'cfg> PackageRegistry<'cfg> { "could not get packages from registry; ids={:?}; ret={:?}", package_ids, ret); - Ok(ret) - } - - pub fn move_sources(self) -> SourceMap<'cfg> { - self.sources + Ok(PackageSet::new(ret, self.sources)) } fn ensure_loaded(&mut self, namespace: &SourceId, kind: Kind) -> CargoResult<()> { diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index b14b4025d..3da7b5e4b 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -2,9 +2,8 @@ use std::default::Default; use std::fs; use std::path::Path; -use core::{Package, PackageSet, Profiles}; -use core::source::{Source, SourceMap}; -use core::registry::PackageRegistry; +use core::{Package, Profiles}; +use core::source::Source; use util::{CargoResult, human, ChainError, Config}; use ops::{self, Layout, Context, BuildConfig, Kind, Unit}; @@ -26,18 +25,7 @@ pub fn clean(manifest_path: &Path, opts: &CleanOptions) -> CargoResult<()> { return rm_rf(&target_dir); } - // Load the lockfile (if one's available) - let lockfile = root.root().join("Cargo.lock"); - let source_id = root.package_id().source_id(); - let resolve = match try!(ops::load_lockfile(&lockfile, source_id)) { - Some(resolve) => resolve, - None => bail!("a Cargo.lock must exist before cleaning") - }; - - // Create a compilation context to have access to information like target - // filenames and such - let srcs = SourceMap::new(); - let pkgs = PackageSet::new(&[]); + let (resolve, packages) = try!(ops::fetch(manifest_path, opts.config)); let dest = if opts.release {"release"} else {"debug"}; let host_layout = Layout::new(opts.config, &root, None, dest); @@ -45,22 +33,16 @@ pub fn clean(manifest_path: &Path, opts: &CleanOptions) -> CargoResult<()> { Layout::new(opts.config, &root, Some(target), dest) }); - let cx = try!(Context::new(&resolve, &srcs, &pkgs, opts.config, + let cx = try!(Context::new(&resolve, &packages, opts.config, host_layout, target_layout, BuildConfig::default(), root.manifest().profiles())); - let mut registry = PackageRegistry::new(opts.config); - // resolve package specs and remove the corresponding packages for spec in opts.spec { + // Translate the spec to a Package let pkgid = try!(resolve.query(spec)); - - // Translate the PackageId to a Package - let pkg = { - try!(registry.add_sources(&[pkgid.source_id().clone()])); - (try!(registry.get(&[pkgid.clone()]))).into_iter().next().unwrap() - }; + let pkg = packages.packages().find(|p| p.package_id() == pkgid).unwrap(); // And finally, clean everything out! for target in pkg.targets() { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index ba7716f12..1b7a5b0ea 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -28,7 +28,7 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use core::registry::PackageRegistry; -use core::{Source, SourceId, SourceMap, PackageSet, Package, Target}; +use core::{Source, SourceId, PackageSet, Package, Target}; use core::{Profile, TargetKind, Profiles}; use core::resolver::{Method, Resolve}; use ops::{self, BuildOutput, ExecEngine}; @@ -102,7 +102,7 @@ pub fn resolve_dependencies<'a>(root_package: &Package, source: Option>, features: Vec, no_default_features: bool) - -> CargoResult<(Vec, Resolve, SourceMap<'a>)> { + -> CargoResult<(PackageSet<'a>, Resolve)> { let override_ids = try!(source_ids_from_config(config, root_package.root())); @@ -134,9 +134,9 @@ pub fn resolve_dependencies<'a>(root_package: &Package, method, Some(&resolve), None)); let packages = try!(ops::get_resolved_packages(&resolved_with_overrides, - &mut registry)); + registry)); - Ok((packages, resolved_with_overrides, registry.move_sources())) + Ok((packages, resolved_with_overrides)) } pub fn compile_pkg<'a>(root_package: &Package, @@ -158,7 +158,7 @@ pub fn compile_pkg<'a>(root_package: &Package, bail!("jobs must be at least 1") } - let (packages, resolve_with_overrides, sources) = { + let (packages, resolve_with_overrides) = { try!(resolve_dependencies(root_package, config, source, features, no_default_features)) }; @@ -180,7 +180,8 @@ pub fn compile_pkg<'a>(root_package: &Package, invalid_spec.join(", ")) } - let to_builds = packages.iter().filter(|p| pkgids.contains(&p.package_id())) + let to_builds = packages.packages() + .filter(|p| pkgids.contains(&p.package_id())) .collect::>(); let mut general_targets = Vec::new(); @@ -245,9 +246,8 @@ pub fn compile_pkg<'a>(root_package: &Package, } try!(ops::compile_targets(&package_targets, - &PackageSet::new(&packages), + &packages, &resolve_with_overrides, - &sources, config, build_config, root_package.manifest().profiles(), diff --git a/src/cargo/ops/cargo_fetch.rs b/src/cargo/ops/cargo_fetch.rs index 23267578f..35002bac1 100644 --- a/src/cargo/ops/cargo_fetch.rs +++ b/src/cargo/ops/cargo_fetch.rs @@ -1,21 +1,25 @@ use std::path::Path; use core::registry::PackageRegistry; -use core::{Package, PackageId, Resolve}; +use core::{Package, PackageId, Resolve, PackageSet}; use ops; use util::{CargoResult, Config, human, ChainError}; /// Executes `cargo fetch`. -pub fn fetch(manifest_path: &Path, config: &Config) -> CargoResult<()> { +pub fn fetch<'a>(manifest_path: &Path, + config: &'a Config) + -> CargoResult<(Resolve, PackageSet<'a>)> { let package = try!(Package::for_path(manifest_path, config)); let mut registry = PackageRegistry::new(config); let resolve = try!(ops::resolve_pkg(&mut registry, &package)); - let _ = try!(get_resolved_packages(&resolve, &mut registry)); - Ok(()) + get_resolved_packages(&resolve, registry).map(move |packages| { + (resolve, packages) + }) } -pub fn get_resolved_packages(resolve: &Resolve, registry: &mut PackageRegistry) - -> CargoResult> { +pub fn get_resolved_packages<'a>(resolve: &Resolve, + registry: PackageRegistry<'a>) + -> CargoResult> { let ids: Vec = resolve.iter().cloned().collect(); registry.get(&ids).chain_error(|| { human("unable to get packages from source") diff --git a/src/cargo/ops/cargo_output_metadata.rs b/src/cargo/ops/cargo_output_metadata.rs index 17b9656f0..bcc9bc70a 100644 --- a/src/cargo/ops/cargo_output_metadata.rs +++ b/src/cargo/ops/cargo_output_metadata.rs @@ -3,7 +3,7 @@ use std::path::Path; use rustc_serialize::{Encodable, Encoder}; use core::resolver::Resolve; -use core::{Source, Package, PackageId}; +use core::{Source, Package, PackageId, PackageSet}; use ops; use sources::PathSource; use util::config::Config; @@ -52,7 +52,7 @@ fn metadata_full(opt: OutputMetadataOptions, config: &Config) -> CargoResult, - no_default_features: bool) - -> CargoResult<(Vec, Resolve)> { +fn resolve_dependencies<'a>(manifest: &Path, + config: &'a Config, + features: Vec, + no_default_features: bool) + -> CargoResult<(PackageSet<'a>, Resolve)> { let mut source = try!(PathSource::for_path(manifest.parent().unwrap(), config)); try!(source.update()); let package = try!(source.root_package()); - let deps = try!(ops::resolve_dependencies(&package, - config, - Some(Box::new(source)), - features, - no_default_features)); - - let (packages, resolve_with_overrides, _) = deps; - - Ok((packages, resolve_with_overrides)) + ops::resolve_dependencies(&package, + config, + Some(Box::new(source)), + features, + no_default_features) } diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 22025b178..40a0b1e6f 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use regex::Regex; -use core::{SourceMap, Package, PackageId, PackageSet, Resolve, Target, Profile}; +use core::{Package, PackageId, PackageSet, Resolve, Target, Profile}; use core::{TargetKind, LibKind, Profiles, Metadata, Dependency}; use core::dependency::Kind as DepKind; use util::{self, CargoResult, ChainError, internal, Config, profile, Cfg, human}; @@ -28,8 +28,8 @@ pub struct Unit<'a> { pub struct Context<'a, 'cfg: 'a> { pub config: &'cfg Config, pub resolve: &'a Resolve, - pub sources: &'a SourceMap<'cfg>, pub compilation: Compilation<'cfg>, + pub packages: &'a PackageSet<'cfg>, pub build_state: Arc, pub build_explicit_deps: HashMap, (PathBuf, Vec)>, pub exec_engine: Arc>, @@ -43,7 +43,6 @@ pub struct Context<'a, 'cfg: 'a> { target_triple: String, target_info: TargetInfo, host_info: TargetInfo, - package_set: &'a PackageSet, profiles: &'a Profiles, } @@ -57,8 +56,7 @@ struct TargetInfo { impl<'a, 'cfg> Context<'a, 'cfg> { pub fn new(resolve: &'a Resolve, - sources: &'a SourceMap<'cfg>, - deps: &'a PackageSet, + packages: &'a PackageSet<'cfg>, config: &'cfg Config, host: Layout, target_layout: Option, @@ -83,13 +81,12 @@ impl<'a, 'cfg> Context<'a, 'cfg> { host: host, target: target_layout, resolve: resolve, - sources: sources, - package_set: deps, + packages: packages, config: config, target_info: target_info, host_info: host_info, compilation: Compilation::new(config), - build_state: Arc::new(BuildState::new(&build_config, deps)), + build_state: Arc::new(BuildState::new(&build_config, packages)), build_config: build_config, exec_engine: engine, fingerprints: HashMap::new(), @@ -213,14 +210,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { /// Returns the appropriate output directory for the specified package and /// target. pub fn out_dir(&self, unit: &Unit) -> PathBuf { - let out_dir = self.layout(unit.pkg, unit.kind); - if unit.target.is_custom_build() { - out_dir.build(unit.pkg) - } else if unit.target.is_example() { - out_dir.examples().to_path_buf() - } else { - out_dir.root().to_path_buf() - } + self.layout(unit.pkg, unit.kind).out_dir(unit.pkg, unit.target) } /// Return the (prefix, suffix) pair for dynamic libraries. @@ -567,7 +557,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { /// Gets a package for the given package id. pub fn get_package(&self, id: &PackageId) -> &'a Package { - self.package_set.iter() + self.packages.packages() .find(|pkg| id == pkg.package_id()) .expect("Should have found package") } diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index bc83ecbdc..552b2d175 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -241,7 +241,7 @@ impl BuildState { pub fn new(config: &super::BuildConfig, packages: &PackageSet) -> BuildState { let mut sources = HashMap::new(); - for package in packages.iter() { + for package in packages.packages() { match package.manifest().links() { Some(links) => { sources.insert(links.to_string(), diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index fcfc6c952..09d3300cc 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -562,7 +562,7 @@ fn dep_info_mtime_if_fresh(dep_info: &Path) -> CargoResult> { fn pkg_fingerprint(cx: &Context, pkg: &Package) -> CargoResult { let source_id = pkg.package_id().source_id(); - let source = try!(cx.sources.get(source_id).chain_error(|| { + let source = try!(cx.packages.sources().get(source_id).chain_error(|| { internal("missing package source") })); source.fingerprint(pkg) diff --git a/src/cargo/ops/cargo_rustc/layout.rs b/src/cargo/ops/cargo_rustc/layout.rs index 6bf8b3e15..1be8ed4df 100644 --- a/src/cargo/ops/cargo_rustc/layout.rs +++ b/src/cargo/ops/cargo_rustc/layout.rs @@ -49,7 +49,7 @@ use std::fs; use std::io; use std::path::{PathBuf, Path}; -use core::Package; +use core::{Package, Target}; use util::Config; use util::hex::short_hash; @@ -154,4 +154,14 @@ impl<'a> LayoutProxy<'a> { pub fn build_out(&self, pkg: &Package) -> PathBuf { self.root.build_out(pkg) } pub fn proxy(&self) -> &'a Layout { self.root } + + pub fn out_dir(&self, pkg: &Package, target: &Target) -> PathBuf { + if target.is_custom_build() { + self.build(pkg) + } else if target.is_example() { + self.examples().to_path_buf() + } else { + self.root().to_path_buf() + } + } } diff --git a/src/cargo/ops/cargo_rustc/links.rs b/src/cargo/ops/cargo_rustc/links.rs index e2a7ad4a8..060941346 100644 --- a/src/cargo/ops/cargo_rustc/links.rs +++ b/src/cargo/ops/cargo_rustc/links.rs @@ -8,7 +8,7 @@ use util::CargoResult; pub fn validate(deps: &PackageSet) -> CargoResult<()> { let mut map: HashMap<_, &PackageId> = HashMap::new(); - for dep in deps.iter() { + for dep in deps.packages() { let lib = match dep.manifest().links() { Some(lib) => lib, None => continue, diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index f76ba09e5..8f2b629de 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -6,7 +6,7 @@ use std::io::prelude::*; use std::path::{self, PathBuf}; use std::sync::Arc; -use core::{SourceMap, Package, PackageId, PackageSet, Target, Resolve}; +use core::{Package, PackageId, PackageSet, Target, Resolve}; use core::{Profile, Profiles}; use util::{self, CargoResult, human}; use util::{Config, internal, ChainError, profile, join_paths}; @@ -51,14 +51,13 @@ pub struct TargetConfig { pub overrides: HashMap, } -pub type PackagesToBuild<'a> = [(&'a Package,Vec<(&'a Target,&'a Profile)>)]; +pub type PackagesToBuild<'a> = [(&'a Package, Vec<(&'a Target,&'a Profile)>)]; // Returns a mapping of the root package plus its immediate dependencies to // where the compiled libraries are all located. pub fn compile_targets<'a, 'cfg: 'a>(pkg_targets: &'a PackagesToBuild<'a>, - deps: &'a PackageSet, + packages: &'a PackageSet<'cfg>, resolve: &'a Resolve, - sources: &'a SourceMap<'cfg>, config: &'cfg Config, build_config: BuildConfig, profiles: &'a Profiles) @@ -78,16 +77,18 @@ pub fn compile_targets<'a, 'cfg: 'a>(pkg_targets: &'a PackagesToBuild<'a>, } }) }).collect::>(); - try!(links::validate(deps)); + try!(links::validate(packages)); let dest = if build_config.release {"release"} else {"debug"}; - let root = deps.iter().find(|p| p.package_id() == resolve.root()).unwrap(); + let root = packages.packages().find(|p| { + p.package_id() == resolve.root() + }).unwrap(); let host_layout = Layout::new(config, root, None, &dest); let target_layout = build_config.requested_target.as_ref().map(|target| { layout::Layout::new(config, root, Some(&target), &dest) }); - let mut cx = try!(Context::new(resolve, sources, deps, config, + let mut cx = try!(Context::new(resolve, packages, config, host_layout, target_layout, build_config, profiles)); -- 2.30.2